ice: add extensible relay candidate providers - #947
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (65.21%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #947 +/- ##
==========================================
+ Coverage 87.94% 88.10% +0.16%
==========================================
Files 46 46
Lines 6495 6507 +12
==========================================
+ Hits 5712 5733 +21
+ Misses 537 531 -6
+ Partials 246 243 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
JoTurk
left a comment
There was a problem hiding this comment.
Is this intended to create a custom transport between the ICE agent and a standard relay? or something else for non standard ICE connections?
If it is only about transporting packets over an arbitrary connection and the interoperability isn't required, couldn't this just be implemented with a custom PacketConn adapter that pipes datagrams over any relay transport without touching ICE itself? we already provides several ways to inject custom network.
Thank you
Thanks for the review. The goal is actually the second case, I think the important distinction is between packet transport and relay candidate gathering.
A custom In the current flow, the candidate, err := NewCandidateRelay(&item.Config)
...
err = a.addCandidate(ctx, candidate, item.Conn)At that point, My use case is different. A non-TURN relay still needs to establish its relay endpoint and provide the information required to construct To be precise, This PR makes only that relay gathering step extensible. The provider returns both the |
|
@bclswl0827 then we just need a way to manually add local candidates? I think that would be cleaner than adding an external gatherer. |
|
I agree that could be a cleaner approach than the current one. If the agent exposed a way to register an externally created local candidate together with its associated PacketConn, the relay allocation could be handled entirely by the application. Something like this: would allow an application to perform its own relay allocation, construct a CandidateRelay, and then hand it over to the ICE agent for registration. Internally, this could reuse the existing addCandidate logic, so the ICE state machine and connectivity checks would remain unchanged. I'm glad to rework the PR in this direction if you think this is a better API. |
21072cb to
cade5b9
Compare
JoTurk
left a comment
There was a problem hiding this comment.
Thank you @bclswl0827 this is much better API than the custom gatherer, just two nits.
| // SPDX-FileCopyrightText: 2026 The Pion community <https://pion.ly> | ||
| // SPDX-License-Identifier: MIT |
There was a problem hiding this comment.
Can you please merge this test file with an existing test file? we're trying to limit how much files we add to /
| func (a *Agent) AddLocalCandidate(cand Candidate, candidateConn net.PacketConn) error { | ||
| if cand == nil { | ||
| return nil | ||
| } | ||
| if candidateConn == nil { | ||
| return ErrCandidatePacketConnNil | ||
| } | ||
|
|
||
| return a.addCandidate(a.loop, cand, candidateConn) | ||
| } |
There was a problem hiding this comment.
Can we make sure that a user error with calling this function with the same candidate twice doesn't cause the agent to close the candidate during duplication checks and just returns an error?
Lines 1358 to 1364 in c649265
Maybe we can just add a new parameter to addCandidate so it errors and returns if it detects duplication, while keeping the close behavior for normal path?
JoTurk
left a comment
There was a problem hiding this comment.
I think this approach is good now, Maybe we can also use this to move all the candidates ownership to ice from webrtc in ice@v5
| // ErrCandidatePacketConnNil indicates a local candidate was added without | ||
| // a packet connection. | ||
| ErrCandidatePacketConnNil = errors.New("candidate packet connection is nil") | ||
|
|
||
| // ErrDuplicateCandidate indicates a local candidate has already been added. | ||
| ErrDuplicateCandidate = errors.New("candidate already added") |
There was a problem hiding this comment.
I think these errors should be private.
|
@Sean-Der what do you think about this API? |
|
@JoTurk I like!
|
|
Maybe Communicate to the user that it is synthetic. |
|
I agree, will change it to that :) |
b86f680 to
988758d
Compare
|
@Sean-Der renamed to AddVirtualCandidate :) |
Allow registering local candidates with an external packet connection.
Allow external relay transports to register candidate providers without adding protocol-specific logic to pion-ice. Expose relay local preference configuration and add tests for custom protocols, multiple providers, and invalid candidate cleanup.
Description
This change adds support for extensible relay candidate providers in pion-ice, which enables use cases where traditional TURN is unavailable or undesirable, such as relaying ICE traffic through application-layer transports.
Currently, relay candidates are tightly coupled with built-in relay mechanisms. This makes it difficult for applications that use custom relay transports (for example, relay over WebSocket, proprietary tunneling protocols, or application-specific forwarding services) to integrate with ICE without modifying pion-ice internals.
This PR introduces a provider interface that allows external transports to register their own relay candidate sources. ICE remains transport-agnostic while applications can provide custom relay implementations.
This also exposes relay local preference configuration so applications can control candidate priority when multiple relay providers are available.
Reference issue
none